Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert PR #418 "Merge branch eni-cleanup into master" #470

Merged
merged 2 commits into from
Sep 20, 2024
Merged

Revert PR #418 "Merge branch eni-cleanup into master" #470

merged 2 commits into from
Sep 20, 2024

Conversation

yash97
Copy link
Contributor

@yash97 yash97 commented Sep 16, 2024 •
edited
Loading

This reverts commit b5a054e.

With this PR CNInode latency creation is gone.

Proof:-

  1. Created two images one with current master branch and another with this PR.
  2. Provisioned extra 50 nodes in cluster and watched creation of nodes and cninodes every 10 seconds.
  3. Results with master branch which has latency:
                                   Tue Sep 17 04:46:10 UTC 2024
	                            having nodes
					50
					having cninode
					12
					Tue Sep 17 04:48:39 UTC 2024
					having nodes
					50
					having cninode
					47


					Tue Sep 17 04:48:50 UTC 2024
					having nodes
					50
					having cninode
					50

As you can see it took nearly two minutes for cninodes to get created after nodes are created.

  1. Result with Image of this branch:
                           Tue Sep 17 05:21:49 UTC 2024
					having nodes
					8
					having cninode
					8
				Tue Sep 17 05:22:12 UTC 2024
					having nodes
					49
					having cninode
					49


				Tue Sep 17 05:22:23 UTC 2024
					having nodes
					50
					having cninode
					50

As you can see here node creation and cni node creation is toe to toe with almost negligible difference.

Issue #, if available:

Description of changes:
reverting pr 418.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@yash97 yash97 requested a review from a team as a code owner September 16, 2024 07:10
@yash97 yash97 changed the title Revert "Merge branch eni-cleanup into master" Revert PR #418 "Merge branch eni-cleanup into master" Sep 16, 2024
@sushrk
Copy link
Contributor

sushrk commented Sep 17, 2024

If we are cutting the next release from release-1.4 branch, do we want to revert this PR from master branch?
Just TOL if it might be easier to sync the release-1.4 branch with internal repo and cherry-pick the commits.

haouc
haouc approved these changes Sep 20, 2024
View reviewed changes
Copy link
Contributor

@haouc haouc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a revert commit. lgtm.

@yash97 yash97 merged commit 471d264 into aws:master Sep 20, 2024
4 checks passed
sushrk added a commit to sushrk/amazon-vpc-resource-controller-k8s that referenced this pull request Nov 13, 2024
@sushrk
Revert "Revert PR aws#418 "Merge branch eni-cleanup into master" (aws…
4d5b86e 
…#470)"

This reverts commit 471d264.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haouc haouc haouc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yash97 @sushrk @haouc